Foolproof mesh-coordinate mutation: capability gate + deform() + SL-field CARRY transfer#246
Merged
Merged
Conversation
… + SL-field CARRY transfer Moving mesh nodes on a live mesh must transfer the user fields AND the semi-Lagrangian/DDt history coherently, or the solution corrupts. The correct machinery (remesh_with_field_transfer + SemiLagrangian.on_remesh ALE pulse) already existed but was only reachable via the metric-driven adapt entries; there was no public way to impose an arbitrary node displacement (what a free surface needs), so drivers fell back to the raw internal Mesh._deform_mesh and hand-rolled an ALE correction that silently skipped the history stack. This makes the correct path the only path. Three changes: 1. Capability gate on Mesh._deform_mesh. Raises (with a directory of the sanctioned entry points) when called bare on a mesh that already carries variables or remesh hooks, outside a sanctioned scope (_in_remesh_transfer, set by the transaction, or a _coord_mutation() scope). Construction / restart-before-vars stays allowed. 2. Public Mesh.deform(new_coords, *, dt=...) and Mesh.ephemeral_coords(). deform() is the missing arbitrary-displacement entry — a thin wrapper over remesh_with_field_transfer (REMAP user fields + fire the on_remesh ALE pulse for the DDt history; dt feeds v_mesh = dx/dt). ephemeral_coords() is a context manager for trusted trial deforms that restores coords on exit. Internal bare callers (submesh sync, FMG / snapshot restore) are wrapped in _coord_mutation(). 3. SNES_AdvectionDiffusion stamps its advected field CARRY + managed-by-DuDt when the DuDt is semi-Lagrangian. The DDt already marked its internal history (psi_star, _workVar, ...) CARRY; the user's field defaulted to REMAP, so on a deform/adapt it was geometrically re-interpolated AND then had v_mesh subtracted in the next trace-back — a double compensation for the mesh motion, inconsistent with the once-compensated CARRY'd history. Routing the field through the single ALE trace-back fixes it (and it is REMAP'd correctly on an OT opt-out reset). Eulerian/Lagrangian fields keep the default policy. Validation: gate unit test passes; the adaptation subset (follow_metric / OT_adapt / advdiff monotone + the deform-poking tests) all pass — the two on_remesh branches are unaffected. The three tests that deliberately exercise the raw primitive (0820/0825/0850) are wrapped in the sanctioned _coord_mutation() scope. Underworld development team with AI support from Claude Code
Contributor
There was a problem hiding this comment.
Pull request overview
This PR makes arbitrary mesh coordinate updates safe and coherent by forcing all “live mesh” coordinate mutations through a transfer-aware path (field remap + SemiLagrangian/DDt history handling), and adjusts adv-diff remesh policy so the user field transfer is consistent with SemiLagrangian history.
Changes:
- Add a capability gate around
Mesh._deform_mesh, plus sanctioned scopes/entry points (_coord_mutation(),deform(),ephemeral_coords()). - Wrap internal coordinate movers and targeted tests in
_coord_mutation()to satisfy the new gate. - When
SNES_AdvectionDiffusionuses SemiLagrangian DDt, stamp the advected field asCARRYand “managed-by” the DDt to avoid double mesh-motion compensation.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_0850_mesh_smoothing.py | Wraps deliberate _deform_mesh usage in the sanctioned _coord_mutation() scope. |
| tests/test_0825_deform_mesh_cache_invalidation.py | Updates deform-cache invalidation tests to use _coord_mutation() around gated primitive calls. |
| tests/test_0820_deform_mesh_solver_rebuild_regression.py | Wraps regression test’s direct _deform_mesh call in _coord_mutation(). |
| src/underworld3/systems/solvers.py | Marks advected field as CARRY + managed-by SL DDt to align field transfer with SL history. |
| src/underworld3/discretisation/discretisation_mesh.py | Adds coordinate-mutation gate, new public APIs (deform, ephemeral_coords), and wraps trusted internal coordinate moves. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
2922
to
2924
| with self._mesh_update_lock: | ||
| coord_vec = self.dm.getCoordinatesLocal() | ||
| coords = coord_vec.array.reshape(-1, self.cdim) |
| intermediate meshes are genuinely ephemeral — only the final | ||
| committed move (via :meth:`deform`) updates fields + history. | ||
| """ | ||
| saved = numpy.asarray(self.X.coords).copy() |
| """ | ||
| from underworld3.discretisation.remesh import remesh_with_field_transfer | ||
|
|
||
| _nc = numpy.asarray(new_coords) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Moving mesh nodes on a live mesh must transfer the user fields and the
semi-Lagrangian / DDt history coherently, or the solution corrupts. The correct
machinery (
remesh_with_field_transfer+SemiLagrangian.on_remeshALE pulse)already existed but was only reachable via the metric-driven adapt entries —
there was no public way to impose an arbitrary node displacement (what a free
surface needs), so drivers fell back to the raw internal
Mesh._deform_meshandhand-rolled an ALE correction that silently skipped the history stack. This makes
the correct path the only path.
Changes
Capability gate on
Mesh._deform_mesh— raises (with a directory of thesanctioned entry points) when called bare on a mesh that already carries
variables or remesh hooks, outside a sanctioned scope (
_in_remesh_transferset by the transaction, or a
_coord_mutation()scope). Construction /restart-before-vars stays allowed.
Public
Mesh.deform(new_coords, *, dt=…)+Mesh.ephemeral_coords()—deform()is the missing arbitrary-displacement entry (thin wrapper overremesh_with_field_transfer: REMAP user fields + fire theon_remeshALEpulse;
dtfeedsv_mesh = Δx/dt).ephemeral_coords()restores coords onexit for trusted trial deforms. Internal bare callers (submesh sync, FMG /
snapshot restore) wrapped in
_coord_mutation().SNES_AdvectionDiffusionstamps its advected field CARRY + managed-by-DuDt(when the DuDt is semi-Lagrangian). The DDt already marked its internal
history (
psi_star, …) CARRY; the user's field defaulted to REMAP, so on adeform/adapt it was geometrically re-interpolated and then had
v_meshsubtracted in the next trace-back — a double compensation for the mesh
motion, inconsistent with the once-compensated CARRY'd history. Routing it
through the single ALE trace-back fixes it (REMAP'd correctly on an OT opt-out
reset). Eulerian/Lagrangian fields keep the default policy.
Validation
_deform_meshon a live mesh raises namingdeform;deform()/ephemeral_coords()work).level_1 and tier_aselection: 227 tests, 0 failures, 0 errors(run per-file). Both
on_remeshbranches clean:follow_metric(standard ALE)and
OT_adapt(opt-out REMAP) pass, as do the advdiff and deform-poking tests.are wrapped in the sanctioned
_coord_mutation()scope.Not in scope
The free-surface convection instability (SL departure foot leaving the receded
domain at downwellings) is not addressed here — this PR only makes the
transfer mechanism coherent and foolproof. That remains separate investigation.
Underworld development team with AI support from Claude Code